Skip to content

Ignore link and file versions in Pnpm 6 package dependencies - #1869

Open
RKS (rksharma-owg) wants to merge 2 commits into
microsoft:mainfrom
rksharma-owg:fix/pnpm6-ignore-link-dependencies
Open

RKS (rksharma-owg) wants to merge 2 commits into
microsoft:mainfrom
rksharma-owg:fix/pnpm6-ignore-link-dependencies

Conversation

@rksharma-owg

Copy link
Copy Markdown

Summary

Ignores link: and file: dependencies when traversing package dependencies in PNPM v6 lockfiles.

Fixes #1555

Problem

In PNPM v6 lockfiles, packages may reference workspace packages or local packages via link: (e.g., link:../pkg) or file: version specifiers.

In Pnpm6Detector.cs:

  1. While yaml.Packages skipped file: prefixes, it did not skip link: prefixes in package paths.
  2. In the second pass where package.Dependencies is traversed to register graph edges, local dependencies (file: and link:) were not skipped.
  3. ReconstructPnpmDependencyPath constructed paths for these local dependencies, but because local packages are not recorded in components, components[pnpmDependencyPath] threw KeyNotFoundException.
  4. This exception was caught and logged as an error by PnpmComponentDetectorFactory.OnFileFoundAsync, terminating dependency edge creation and leaving graph construction incomplete.

Solution

  1. In Pnpm6Detector.cs, skip package keys starting with PnpmConstants.PnpmLinkDependencyPath in addition to PnpmConstants.PnpmFileDependencyPath.
  2. In Pnpm6Detector.cs, check this.pnpmParsingUtilities.IsLocalDependency(new KeyValuePair<string, string>(name, version)) before attempting to reconstruct and look up dependencies in components.
  3. Bump PnpmComponentDetectorFactory.Version from 8 to 9.

Tests

  • Added TestPnpmDetector_V6_IgnoresLinkAndFilePackageDependenciesAsync to PnpmDetectorTests:
    • Verifies lockfiles with link: and file: entries under package.Dependencies parse without throwing KeyNotFoundException.
    • Verifies that subsequent non-local dependencies and graph edges are registered correctly.
  • Confirmed all 22 tests in PnpmDetectorTests pass.

In the PNPM v6 lockfile format, package dependencies may reference local
or workspace packages using link: or file: version paths.

Previously, Pnpm6Detector only ignored file: packages in top-level package
keys and did not check for local dependencies (file: or link:) when traversing
package.Dependencies. This led to KeyNotFoundException lookups in the
components map and failed dependency graph construction.

This change:
1. Skips both file: and link: dependency paths when discovering packages in yaml.Packages.
2. Skips local package dependencies (file: and link:) when building dependency edges.
3. Bumps PnpmComponentDetectorFactory.Version from 8 to 9.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Regression coverage should include local package entries under packages.

Pull request overview

Updates PNPM v6 detection to ignore local link: and file: dependencies while preserving graph construction.

Changes:

  • Filters local package paths and dependency references.
  • Bumps the detector version from 8 to 9.
  • Adds regression coverage for local dependencies.
File summaries
File Description
test/Microsoft.ComponentDetection.Detectors.Tests/PnpmDetectorTests.cs Tests PNPM v6 local dependency handling.
src/Microsoft.ComponentDetection.Detectors/pnpm/PnpmComponentDetectorFactory.cs Increments detector version.
src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm6Detector.cs Filters local package paths and dependency edges.
Review details

Suppressed comments (2)

src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm6Detector.cs:29

  • The new test exercises file:/link: values in a package's dependency map, but it never puts a file: or link: entry under packages. Consequently, removing this newly added package-key guard would still leave the test green, so the first part of the fix is not regression-tested; add local package entries and assert they are excluded from the detected components.
            if (pnpmDependencyPath.StartsWith(PnpmConstants.PnpmFileDependencyPath) || pnpmDependencyPath.StartsWith(PnpmConstants.PnpmLinkDependencyPath))

test/Microsoft.ComponentDetection.Detectors.Tests/PnpmDetectorTests.cs:601

  • This fixture covers the new IsLocalDependency guard for package.Dependencies values, but it never includes a link: key under packages. The new first-pass condition in Pnpm6Detector is therefore untested and could regress while this test still passes; add a representative local package entry and keep asserting that it is not detected.
packages:
  /[email protected]:
    resolution: {integrity: sha512-mock=}
    dependencies:
      pkg-link: link:../pkg-link
  • Files reviewed: 3/3 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot AI review requested due to automatic review settings September 17, 2026 03:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No unresolved review issues were identified.

Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pnpm 6 Detector should also ignore link: versions in package dependencies

2 participants